Configure GitHub pages with docsify#43
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a Docsify-based documentation site and site build artifacts, updates Codecov configuration to a component-based layout, adjusts CI workflows for docs and build triggers, modifies alarm readiness API to require explicit registration, and makes minor PR template, README, and inline-comment changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Startup as startup_background.ts
participant Background as background.ts
participant Heart as AlarmHeart
participant Messenger as messenger.alarms
rect rgba(240,248,255,0.9)
Note over Startup,Heart: Old flow (pre-change)
Startup->>Heart: assumeReady()
Heart->>Heart: isListenerReady = true
end
rect rgba(245,255,240,0.9)
Note over Background,Messenger: New flow (post-change)
Background->>Background: define onAlarm callback (top-level)
Background->>Messenger: register onAlarm listener
Background->>Heart: heart.ready(onAlarm)
Heart->>Heart: set readiness based on registered callback
Messenger->>Background: onAlarm event
Background->>Background: handle/log alarm
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
.github/site/index.html (1)
1-11: Consider adding SRI hashes for CDN resources.CDN resources are loaded without Subresource Integrity (SRI) hashes. While this is common for documentation sites, adding SRI would protect against CDN compromises.
You can generate SRI hashes using:
#!/bin/bash # Generate SRI hashes for CDN resources echo "Docsify CSS:" curl -s "https://cdn.jsdelivr.net/npm/docsify@4.13.1/lib/themes/vue.min.css" | openssl dgst -sha384 -binary | openssl base64 -A echo -e "\n\nMermaid JS:" curl -s "https://cdn.jsdelivr.net/npm/mermaid@11.12.0/dist/mermaid.min.js" | openssl dgst -sha384 -binary | openssl base64 -A echo -e "\n\nDocsify JS:" curl -s "https://cdn.jsdelivr.net/npm/docsify@4.13.1/lib/docsify.min.js" | openssl dgst -sha384 -binary | openssl base64 -A echo -e "\n\nDocsify Search:" curl -s "https://cdn.jsdelivr.net/npm/docsify@4.13.1/lib/plugins/search.min.js" | openssl dgst -sha384 -binary | openssl base64 -AThen add
integrityandcrossoriginattributes to each resource.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/codecov.yml(1 hunks).github/pull_request_template.md(1 hunks).github/site/coverpage.md(1 hunks).github/site/homepage.md(1 hunks).github/site/index.html(1 hunks).github/site/index.mjs(1 hunks).github/workflows/build.yml(1 hunks).github/workflows/doc.yml(1 hunks)README.md(1 hunks)src/app-background/compose_action_notifier.ts(1 hunks)src/app-background/menu_handler.ts(1 hunks)src/root/background.ts(3 hunks)src/root/startup/startup_background.ts(0 hunks)src/thunderbird/background_util/alarm_heart.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/root/startup/startup_background.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/root/background.ts (2)
src/root/startup/startup_background.ts (1)
prepareBackgroundRouter(33-37)src/thunderbird/messenger/alarms.ts (1)
Alarm(39-46)
src/thunderbird/background_util/alarm_heart.ts (2)
src/ghosttext-runner/api.ts (1)
IHeart(11-17)src/thunderbird/messenger/alarms.ts (1)
Alarm(39-46)
🪛 LanguageTool
.github/site/homepage.md
[uncategorized] ~43-~43: The official name of this software platform is spelled with a capital “H”.
Context: ...e [Thunderbird add-on website][amo] or [Github Releases][rels]. 2. In Thunderbird, go ...
(GITHUB)
[style] ~91-~91: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...header). * [Rate the add-on][review]. * Feel free to open issues and submit pull requests. S...
(FEEL_FREE_TO_STYLE_ME)
🪛 markdownlint-cli2 (0.18.1)
.github/site/coverpage.md
13-13: Link fragments should be valid
(MD051, link-fragments)
.github/site/homepage.md
62-62: Images should have alternate text (alt text)
(MD045, no-alt-text)
63-63: Images should have alternate text (alt text)
(MD045, no-alt-text)
64-64: Images should have alternate text (alt text)
(MD045, no-alt-text)
127-127: Link fragments should be valid
(MD051, link-fragments)
137-137: Link and image reference definitions should be needed
Unused link or image reference definition: "proj"
(MD053, link-image-reference-definitions)
149-149: Link and image reference definitions should be needed
Unused link or image reference definition: "protocol"
(MD053, link-image-reference-definitions)
🔇 Additional comments (18)
src/app-background/menu_handler.ts (1)
19-20: Clear documentation of design rationale.The added comments provide helpful context about why the initialization logic uses a simple approach rather than more sophisticated alternatives, and specifically calls out real corner cases (like UI language changes). This is useful for future maintainers who might otherwise attempt to "improve" this logic.
src/app-background/compose_action_notifier.ts (1)
4-4: Clear and appropriate documentation.The added JSDoc comment accurately describes the class responsibility. No functional changes.
.github/workflows/build.yml (1)
5-5: Branch filter update looks reasonable.The pattern change from
wip/**to['coderabbit**', 'gh-**']appropriately excludes CodeRabbit auto-fix branches and GitHub Pages-related branches from CI runs. This aligns with avoiding CI overhead on automated branches..github/pull_request_template.md (1)
1-10: PR template improvements are appropriate.The additions enhance governance: the dual-licensing acknowledgment ensures contributor awareness, and the CodeRabbit directive enables automated reviews. These changes align with the PR's broader infrastructure updates.
README.md (1)
271-277: External link updates improve clarity and reliability.The license badge links now point to official license pages, and the Thunderbird reference link directs to the ESR download page—both improvements that enhance user experience and align with the documented ESR requirement.
.github/codecov.yml (1)
1-18: Component-based coverage tracking improves observability.The new
component_managementconfiguration provides granular coverage tracking across app, ghosttext, and infra components. The path glob patterns clearly delineate responsibility areas, and setting project/patch statuses to informational prevents false negatives on coverage..github/workflows/doc.yml (1)
53-57: The review comment is incorrect; all required files and directories exist.The workflow steps on lines 53-57 will execute successfully:
Line 54 (
mv build/doc/ ...): Thebuild/doc/directory is generated at runtime by theyarn run docstep (line 48), which runs TypeDoc. This is standard TypeDoc behavior and the directory will exist when line 54 executes.Line 57 (
mv *.md doc/ ext/ ...): All sources exist in the repository:
- Markdown files: BUILDING.md, CONTRIBUTING.md, README.md ✓
doc/directory: contains documentation ✓ext/directory: contains extension assets ✓The workflow trigger (lines 7-10) only activates when these paths change, so they are guaranteed to exist during execution. No defensive flags or conditional checks are needed.
Likely an incorrect or invalid review comment.
src/root/background.ts (3)
7-7: LGTM: Import added for type safety.The
Alarmtype import supports the new typedonAlarmfunction signature.
38-40: LGTM: Top-level alarm handler.The top-level
onAlarmfunction correctly satisfies MV3 requirements for event listener registration.
62-62: Listener registration is correct but see ordering issue above.The top-level registration satisfies MV3 requirements, but the
ready()call ordering needs to be fixed (see comment on lines 32-33)..github/site/index.mjs (5)
1-1: Verify Mermaid is loaded before this script executes.This assumes
window.mermaidexists. Given the script loading order issue inindex.html, ensure this script executes aftermermaid.min.jsloads (see comment onindex.html:14-17).
5-15: Note:executeScript: truesecurity consideration.Setting
executeScript: trueallows JavaScript in markdown code blocks to execute. Ensure all documentation content is from trusted sources to prevent XSS risks.
22-30: LGTM: Search index configuration.The search paths cover key documentation files appropriately.
31-40: Consider escaping HTML entities in Mermaid code blocks.The
codeparameter is directly interpolated into HTML without escaping. While the documentation is from trusted sources, escaping would prevent issues if code contains</pre>or other HTML.markdown: { renderer: { code(code, lang) { if (lang === "mermaid") { - return `<pre class="mermaid">${code}</pre>`; + // Escape HTML entities to prevent breaking out of <pre> + const escaped = code.replace(/[&<>"']/g, (c) => { + const escapes = { '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }; + return escapes[c]; + }); + return `<pre class="mermaid">${escaped}</pre>`; } return this.origin.code.apply(this, arguments);Wait, actually Mermaid needs to parse the raw code, so we shouldn't escape it. Mermaid will handle the rendering safely. Let me reconsider...
Actually, looking at Mermaid documentation, the recommended way is to put the code inside a
<pre class="mermaid">tag, and Mermaid will safely process it. So the current implementation is correct. Mermaid itself handles any potential issues with the content.
41-45: LGTM: Mermaid plugin integration.The plugin correctly triggers Mermaid rendering after each route loads.
src/thunderbird/background_util/alarm_heart.ts (3)
2-4: LGTM: Import and documentation added.The
Alarmtype import and doc comment correctly describe the class purpose.
11-17: LGTM: Improved readiness verification.The
ready()method correctly verifies that the callback is actually registered usinghasListener(), which is more robust than the previousassumeReady()approach. The MV3 requirement is well-documented.
19-26: LGTM: Assertion enforces correct initialization.The assertion at line 20 will catch initialization errors (such as the ordering issue in
background.ts), which is good defensive programming.
* Fix links to license website * Link to the Thunderbird ESR download page
* Also remove catch-all `src/` definition from the Codecov config
snipsnipsnip
force-pushed
the
docsify-website
branch
from
9d188da to
c7e64b7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/site/homepage.md (1)
43-43: Capitalize "GitHub" per style guidelines.Line 43 uses "Github" but the official product name is "GitHub" with a capital "H".
- 1. Download the latest release from the [Thunderbird add-on website][amo] or [Github Releases][rels]. + 1. Download the latest release from the [Thunderbird add-on website][amo] or [GitHub Releases][rels]..github/site/coverpage.md (1)
13-13: Fix mismatched link text and target anchor.Line 13 displays text "Installation" but links to
#requirementsinstead of#installation. The displayed text should match the destination anchor.-[Installation](#requirements) +[Installation](#installation)
🧹 Nitpick comments (1)
.github/site/homepage.md (1)
137-137: Remove unused link reference definition.Line 137 defines
[proj]which is never referenced in the markdown content. Removing it reduces clutter.-[proj]: https://github.com/exteditor/ghostbird/projects [milestones]: https://github.com/exteditor/ghostbird/milestones
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/codecov.yml(1 hunks).github/site/coverpage.md(1 hunks).github/site/homepage.md(1 hunks).github/site/index.html(1 hunks).github/site/index.mjs(1 hunks).github/workflows/build.yml(1 hunks).github/workflows/doc.yml(1 hunks)README.md(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/build.yml
- .github/codecov.yml
- .github/site/index.mjs
🧰 Additional context used
🪛 LanguageTool
.github/site/homepage.md
[uncategorized] ~43-~43: The official name of this software platform is spelled with a capital “H”.
Context: ...e [Thunderbird add-on website][amo] or [Github Releases][rels]. 2. In Thunderbird, go ...
(GITHUB)
[style] ~91-~91: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...header). * [Rate the add-on][review]. * Feel free to open issues and submit pull requests. S...
(FEEL_FREE_TO_STYLE_ME)
🪛 markdownlint-cli2 (0.18.1)
.github/site/coverpage.md
13-13: Link fragments should be valid
(MD051, link-fragments)
.github/site/homepage.md
137-137: Link and image reference definitions should be needed
Unused link or image reference definition: "proj"
(MD053, link-image-reference-definitions)
🔇 Additional comments (4)
README.md (3)
65-67: Excellent accessibility improvements to button images.Adding explicit
altattributes with descriptive text ("Gray button", "Blue button", "Red button") improves screen reader support and follows WCAG best practices. The alt text is concise and contextually useful for users navigating with assistive technologies.
271-272: Updated license badge links to official sources.The links now correctly point to the official Mozilla Public License 2.0 and GNU GPL-3.0 license pages, replacing any outdated or redirect URLs. This ensures users can easily access authoritative license information.
277-277: Thunderbird link now points to official ESR download page.The reference link correctly directs to the official Thunderbird ESR download page, which aligns with the requirement stated in line 15 specifying Thunderbird 140 ESR support.
.github/site/index.html (1)
11-11: Verify that Docsify configuration is set before library initialization.Based on a past review, there was a critical issue with script loading order where
window.$docsifyconfiguration was set after Docsify's library loaded. The current file shows only the module script; please verify thatindex.mjsdynamically loads Docsify in the correct order, or that configuration is inline before the library script.If
index.mjsloads Docsify dynamically, ensure it setswindow.$docsifybefore creating and appending the Docsify script tag. If you're loading Docsify from a CDN, it should appear after the configuration is set in the DOM.Can you confirm that the Docsify library is loaded after configuration is defined?
snipsnipsnip
force-pushed
the
docsify-website
branch
from
c7e64b7 to
7bb1fbf
Compare
github-project-automation
bot
moved this from 📥️Inbox
to 🪺Experimental
in Ghostbird Issues
1 participant
Closes #5
Summary by CodeRabbit